ref: sort options obj and remove optional params #1545
Conversation
🦋 Changeset detectedLatest commit: 63b9db6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
JonasBa
force-pushed
the
jb/ref/config-argv
branch
from
cf6bad6 to
63b9db6
Compare
|
I haven't taken the time to understand what these changes are for; but will add one comment would be that given the size of the diff, this will generate a lot of work to merge other large open PRs, e.g. #1475 |
|
@eoghanmurray lets skip it then and I will reopen it when your PR is merged. The idea is to reduce the number of argument shapes passed to the function which allows JS engines to make certain optimizations they otherwise wouldnt be able to. |
|
Thanks @JonasBa for the tricky work and hope we can revisit. |
|
I'd personally be in favour of omitting options from the whole typescript system and just checking for their existence (and type) on a generic option object, but I'd say that's just me! |
Feel free to discard this PR, it's more of a suggestion than anything else and would require a new semver major due to the breaking API change, but I found it hard to track the options object around the different call sites (especially as a lot of field are optional and it's hard to know exactly what is being passed vs what is default initialized).
This PR changes that by removing changing all optional field of the serialize options params and sorting it. The goal of the change is to simplify the control flow by not having to look at what is default initialized as well as hopefully reducing the polymorphic argument values.